Faster svg pan #2623
Faster svg pan #2623
Conversation
- (x|y) -> (x|y)Scale to match Drawing.setTextPointsScale - pull out regex from function scope into module scope - do not return the scale (it is not used anywhere)
... as opposed to on every mousemove.
... and apply inverse scale on pts only when needed! - stash selections on the subplot plotinfo object - stash 'old' (x|y) scale factor to determine if we need to update all the pts and text nodes - rename 'draggedPts' category -> 'zoomScale' - make Drawing.hideOutsideRangePoints handle bar vs scatter differences internally.
|
|
||
| selection.each(function() { | ||
| // Get the transform: | ||
| t = (this.getAttribute('transform') || '').replace(re, ''); | ||
| var t = (this.getAttribute('transform') || '').replace(SCALE_RE, ''); | ||
| t += scale; | ||
| t = t.trim(); |
There was a problem hiding this comment.
I don't know whether the string manipulations here are a meaningful contribution to the performance... but this could all be simplified a bit (indexOf & .substr instead of a regex & .replace, no .trim) if we could omit the space before the word scale. It seems to work fine in my browsers, though in principle it's not supposed to be allowed: "The individual transform definitions are separated by whitespace and/or a comma."
Alternatively, we could imagine stashing the pre-scaled transform as perhaps a data-initialtransform attribute on each element and avoid all this processing...
Not necessary, just a possibility if you think there would be a benefit.
There was a problem hiding this comment.
I haven't benchmarked this particular block yet, but I know that Drawing.setTextPointsScale is 4-5 times slower than this routine here Drawing.setPointGroupScale.
Spending time on making these routines faster would help improve scroll performance. So yeah, thanks for writing this down. I just opened a new ticket about this -> #2625
|
@etpinard looks great! Just the one nonblocking comment, lets do it. 💃 |
resolves #2548 (there's still room for improvement, but it's enough to close that ticket for now I think).
In brief this PR:
transformattr of every point on every mousemove to counteract the scale of trace group during pan/scroll. It turns out that can be extremely costly on graphs with many pts and even more costly for text nodes.plots/cartersian/dragbox.jsis now free of (potentially) slowselectAllcalls!hasSVG,hasScattergland friends only on mousedown instead of on every mousemove.Taking https://plot.ly/~jackp/18395.embed to benchmark things:
before each mousedown interval took around 30-40ms:
off this branch we're down in around 1ms :
Note that this PR improves scroll performance a little as stashing shaves about 5ms per scroll interval, but is still pretty slow (we might have to wait for scattergl text there #2501).
cc @alexcjohnson @jackparmer